src/commands/rooms/presence/get-all.ts (1)

123-126: Keep the empty-state warning on stdout.

This is the only warning path in the command that goes to stderr; the pagination warnings in the same file already use stdout, so rooms presence get-all ends up behaving differently from the rest of the CLI for human-mode warnings.

♻️ Suggested change

-      } else if (items.length === 0) {
-        this.logToStderr(
-          formatWarning("No members currently present in this room."),
-        );
+      } else if (items.length === 0) {
+        this.log(formatWarning("No members currently present in this room."));
       } else {

Based on learnings, warnings produced by formatWarning(...) in src/commands/** should be emitted via this.log(...) on stdout when human output is enabled.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/commands/rooms/presence/get-all.ts` around lines 123 - 126, In the
empty-state branch inside the rooms presence get-all command, replace the stderr
emission with stdout: change the call that uses
this.logToStderr(formatWarning("No members currently present in this room.")) to
emit via this.log(...) so the formatted warning follows the same human-mode
stdout pattern as other pagination warnings; locate the branch that checks
items.length === 0 and update the call to use this.log with formatWarning to
maintain consistent CLI behavior.

test/unit/commands/rooms/presence/enter.test.ts (1)

99-151: Assert on runCommand() output here instead of spying on console.log.

runCommand() already captures oclif output for unit tests, so this adds a second capture path and makes the test more fragile than the rest of the suite.

♻️ Suggested change

-      const capturedLogs: string[] = [];
-
-      const logSpy = vi.spyOn(console, "log").mockImplementation((msg) => {
-        capturedLogs.push(String(msg));
-      });
-
       let presenceCallback: ((event: unknown) => void) | null = null;
       room.presence.subscribe.mockImplementation((callback) => {
         presenceCallback = callback;
         return { unsubscribe: vi.fn() };
       });
@@
-      await commandPromise;
-      logSpy.mockRestore();
-
-      const output = capturedLogs.join("\n");
+      const { stdout } = await commandPromise;
+      const output = stdout;

Based on learnings, unit tests in this repo should assert stdout from runCommand() and reserve console.log spies for integration/e2e coverage.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/unit/commands/rooms/presence/enter.test.ts` around lines 99 - 151,
Remove the console.log spy and assert on the oclif-captured output from
runCommand instead: stop creating capturedLogs and vi.spyOn(console,"log"), keep
the mock room and presence.subscribe setup and the presenceCallback event
simulation, but call and await runCommand(...) to get its stdout (e.g., const
output = await runCommand(["rooms:presence:enter","test-room","--show-others"],
import.meta.url)), then assert output contains "other-user" and does not contain
mock.clientId; also remove logSpy.mockRestore() and any capturedLogs.join usage.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

README.md (1)

4252-4276: ⚠️ Potential issue | 🟡 Minor

Add language tags to these fenced command blocks (MD040).
Both new fenced blocks are still missing a language identifier, so markdownlint will keep failing.

Suggested patch

-```
+```text
 USAGE
   $ ably rooms presence get-all ROOM [-v] [--json | --pretty-json] [--limit <value>]
@@
   $ ably rooms presence get-all my-room --pretty-json

@@
- +text
USAGE
$ ably rooms presence update ROOM --data [-v] [--json | --pretty-json] [--client-id ] [-D ]
@@
$ ably rooms presence update my-room --data '{"status":"online"}' --duration 60

Also applies to: 4316-4344

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 4252 - 4276, The markdown fenced code blocks for the
CLI examples (the blocks showing "USAGE ... ably rooms presence get-all ..." and
the block for "ably rooms presence update ...") lack a language identifier and
trigger MD040; update each opening fence from ``` to ```text for the two shown
examples (and the other occurrences noted around the presence update example and
the duplicate region) so the blocks are explicitly marked as plain text and
markdownlint stops flagging them.

test/unit/commands/rooms/presence/enter.test.ts (1)

104-151: Use stdout from runCommand() result instead of console.log spying for consistency with other tests.

This test manually spies on console.log to capture presence event output, but the pattern in similar streaming tests (e.g., rooms:typing:subscribe) shows that runCommand() properly captures all this.log() calls in stdout. Simply await the command and use the result's stdout property, which is cleaner and aligns with repository standards.

Suggested refactor

-      const capturedLogs: string[] = [];
-
-      const logSpy = vi.spyOn(console, "log").mockImplementation((msg) => {
-        capturedLogs.push(String(msg));
-      });
-
       let presenceCallback: ((event: unknown) => void) | null = null;
       room.presence.subscribe.mockImplementation((callback) => {
         presenceCallback = callback;
         return { unsubscribe: vi.fn() };
       });
 
-      const commandPromise = runCommand(
+      const result = await runCommand(
         ["rooms:presence:enter", "test-room", "--show-others"],
         import.meta.url,
       );
 
       await vi.waitFor(
         () => {
           expect(room.presence.subscribe).toHaveBeenCalled();
         },
         { timeout: 1000 },
       );
 
       // Simulate a self event (should be filtered out)
       if (presenceCallback) {
         presenceCallback({
           type: "enter",
           member: {
             clientId: mock.clientId,
             data: {},
           },
         });
 
         // Simulate another user's event (should be shown)
         presenceCallback({
           type: "enter",
           member: {
             clientId: "other-user",
             data: {},
           },
         });
       }
-
-      await commandPromise;
-      logSpy.mockRestore();
-
-      const output = capturedLogs.join("\n");
+      const output = result.stdout;
       expect(output).toContain("other-user");
       expect(output).not.toContain(mock.clientId);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/unit/commands/rooms/presence/enter.test.ts` around lines 104 - 151,
Remove the manual console.log spy and capturedLogs logic and instead await the
runCommand(...) call to get its result object, use room.presence.subscribe mock
to capture presenceCallback as before, invoke presenceCallback with the self and
other-user events, then await the runCommand result and assert against
result.stdout (expect it to contain "other-user" and not contain mock.clientId);
update references to runCommand, room.presence.subscribe, presenceCallback, and
mock.clientId accordingly.

src/commands/rooms/presence/enter.ts:70

• --data "{}" is currently treated as “no data” (rawData !== "{}"), which makes this.data stay null even though the command will still enter presence with a non-null payload (see the presence.enter(this.data || {}) call below). This can lead to the CLI reporting data: null in JSON/human output while the actual presence data is {}. Consider removing the special-case for "{}" and always parsing provided JSON, including an empty object, so output reflects the actual presence payload.

    const rawData = flags.data;
    if (rawData && rawData !== "{}") {
      const parsed = this.parseJsonFlag(rawData, "data", flags);
      this.data = parsed as PresenceData;
    }

src/commands/rooms/presence/subscribe.ts (1)

115-117: Consider removing the redundant timestamp line.

Line 116 outputs a visual timestamp via formatTimestamp(timestamp), and line 117 immediately outputs the same timestamp with a label. This results in the timestamp appearing twice in the output. If the visual separator is intentional, consider adding a brief comment explaining the purpose.

♻️ Proposed fix to remove redundancy

         const lines: string[] = [
-          formatTimestamp(timestamp),
           `${formatLabel("Timestamp")} ${timestamp}`,
           `${formatLabel("Action")} ${formatEventType(event.type)}`,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/commands/rooms/presence/subscribe.ts` around lines 115 - 117, The lines
array currently contains both formatTimestamp(timestamp) and
`${formatLabel("Timestamp")} ${timestamp}`, causing the same timestamp to appear
twice; open the const lines definition in subscribe.ts (look for formatTimestamp
and formatLabel usage) and remove the redundant entry (prefer keeping the visual
formatTimestamp(timestamp) and delete the `${formatLabel("Timestamp")}
${timestamp}` line), or if the visual separator is intentional keep both but add
a short comment explaining why both are present.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes